Skip to content

Comments

fix: handle case when tally is equal in common_highest_checkpoint#1239

Merged
KolbyML merged 1 commit intoReamLabs:masterfrom
shariqnaiyer:fix/shariqnaiyer/late-joiner-1
Feb 23, 2026
Merged

fix: handle case when tally is equal in common_highest_checkpoint#1239
KolbyML merged 1 commit intoReamLabs:masterfrom
shariqnaiyer:fix/shariqnaiyer/late-joiner-1

Conversation

@shariqnaiyer
Copy link
Contributor

@shariqnaiyer shariqnaiyer commented Feb 23, 2026 •
edited
Loading

What was wrong?

When comparing checkpoints from peers. We tally peers but in case of a tie, we should choose the checkpoint with the highest slot.

This should fix the late joiner test on Devnet3.

How was it fixed?

let mut highest_tally = 0;
        for (checkpoint, tally) in checkpoint_tally {
            if tally > highest_tally
                || (tally == highest_tally
                    && common_checkpoint
                        .is_some_and(|common_checkpoint| checkpoint.slot > common_checkpoint.slot))
            {
                highest_tally = tally;
                common_checkpoint = Some(checkpoint);
            }
        }

        common_checkpoint

I initially expanded the if statement which looked a little ugly to read and found a clean way to do the loop.

Tuples in Rust are compared left to right so it's equivalent to the above loop.

To-Do

@shariqnaiyer shariqnaiyer changed the title fix: handle case when tally is equal common_highest_checkpoint fix: handle case when tally is equal in common_highest_checkpoint Feb 23, 2026
@KolbyML
Copy link
Contributor

KolbyML commented Feb 23, 2026

I need to look more into this

KolbyML
KolbyML approved these changes Feb 23, 2026
View reviewed changes
Copy link
Contributor

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@KolbyML KolbyML merged commit 6f8c567 into ReamLabs:master Feb 23, 2026
22 checks passed
Kayden-ML
Kayden-ML reviewed Feb 23, 2026
View reviewed changes
Copy link
Contributor

@Kayden-ML Kayden-ML left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

Patchoulis pushed a commit to Patchoulis/ream that referenced this pull request Feb 25, 2026
@shariqnaiyer @Patchoulis
fix: handle case when tally is equal in common_highest_checkpoint (Re…
9168eaa 
…amLabs#1239)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kayden-ML Kayden-ML Kayden-ML left review comments

@KolbyML KolbyML KolbyML approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shariqnaiyer @KolbyML @Kayden-ML